Skip to content

test options for custom icons and labels - #4981

Merged
haroun merged 5 commits into
mainfrom
pro-7647-custom-meta-icon
Jul 7, 2025
Merged

test options for custom icons and labels#4981
haroun merged 5 commits into
mainfrom
pro-7647-custom-meta-icon

Conversation

@haroun

@haroun haroun commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

Summary

Allow to customize icons and labels for AI

What are the specific steps to test this change?

This requires

  1. Enable @apostrophecms-pro/seo-assistant and @apostrophecms-pro/automatic-translation following the doc https://github.com/apostrophecms/seo-assistant/
  2. Update modules/@apostrophecms/i18n/i18n/en.json
    {
      "automaticTranslationCheckboxHelp": "By clicking the below checkbox, you: <br />(a) acknowledge that you are enabling users within your organization to use the iCIMS Copilot Preview Feature(s); <br />(b) acknowledge that you understand that the ¡CIMS Copilot is a Preview Feature, and its availability and use is governed by the Preview Feature Terms and Conditions; <br />(c) have read, understand, and agree to be bound by the Preview Feature Terms and Conditions; and <br />(d) represent and warrant that you have the right, power, and authority to bind your organization. For more information on Preview Features, please see Understanding Preview Features in the iCIMS Talent Cloud. If you do not agree to the Preview Feature Terms and Conditions, do not click the below checkbox. You may still click Localize Content and be able to translate your landing pages without iCIMS Copilot.",
      "automaticTranslationDisclaimer": "Translations are produced by AI. They can be inaccurate, omit nuance, or introduce formatting issues. Review all translated content before you publish."
    }
    
  3. Update modules/@apostrophecms-pro/automatic-translation/i18n/en.json
    {
      "fieldMeta": "Translated with Copilot"
    }
    
  4. Update modules/@apostrophecms-pro/seo-assistant/index.js
    export default {
      icons: {
        'panda-icon': 'Panda'
      },
      options: {
        provider: 'openai',
        triggerAssistantButton: {
          icon: 'panda-icon',
          label: 'Generate with Copilot',
          class: [ 'blue-yellow' ]
        }
      }
    };
    
  5. Update modules/@apostrophecms/seo-assistant-doc-type/index.js
    export default {
      options: {
        seoAssistantMetaPosition: 'right'
      }
    };
    
  6. Update your style modules/asset/ui/src/index.scss with
    .apos-button.apos-button--blue-yellow {
      background-color: yellow;
      border: 4px solid blue;
    }
    
  7. Edit a document and go to the SEO tab, you should see this image
  8. Localize a document, you should see this image and check the box Translate text content
  9. Go the translated document, edit the same document, you should see this image

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@haroun haroun self-assigned this Jun 13, 2025
@linear

linear Bot commented Jun 13, 2025

Copy link
Copy Markdown

@haroun
haroun requested review from boutell and myovchev June 18, 2025 07:58
@haroun
haroun marked this pull request as ready for review June 18, 2025 08:00

@myovchev myovchev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the comment on the checkbox item help vs htmlHelp.

<p
v-if="(choice.help || choice.htmlHelp)"
class="apos-choice-label-text--help apos-field__help"
v-html="$t(choice.help || choice.htmlHelp)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choice help should not be rendered as HTML unlike htmlHelp. I think you need two separate entries and checks for this render.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was carried over from AposInputWrapper.vue#105.

value: wizard.values.translateContent.data,
label: $t('apostrophe:automaticTranslationCheckbox')
label: $t('apostrophe:automaticTranslationCheckbox'),
htmlHelp: $t('apostrophe:automaticTranslationCheckboxHelp')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that i18n is not encoding HTML?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do it already and it seems to work, for example i18n/en.json#54

@myovchev

Copy link
Copy Markdown
Contributor

I also forgot to mention - we need a CHANGELOG entry explaining that choice.help and choice.htmlHelp is a thing for the checkbox component only.

@haroun
haroun requested review from myovchev and stuartromanek June 26, 2025 14:33
myovchev
myovchev previously approved these changes Jun 26, 2025

@stuartromanek stuartromanek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense for them to change or confuse the modifiers prop

@haroun

haroun commented Jun 30, 2025

Copy link
Copy Markdown
Contributor Author

I don't think it makes sense for them to change or confuse the modifiers prop

I tried to match the screenshots in the original ticket https://apostrophetechnologies.zendesk.com/agent/tickets/1317

@stuartromanek

Copy link
Copy Markdown
Member

I don't think it makes sense for them to change or confuse the modifiers prop

I tried to match the screenshots in the original ticket https://apostrophetechnologies.zendesk.com/agent/tickets/1317

I just don't think it makes sense. What those modifiers do is not documented or previewable. Why don't we give them a prop to apply a class to the overridden element so that they can further refine it in their own front-end?

* main:
  Fix login tests for whoami route. (#4991)
  Implemented API endpoint to return current users info (#4872)
  Fix field options and allow import image tags (#4988)
  Fix wrapper class when no base class is set (#4986)
  Ensure defaultNode has highest priority (#4985)
  PRO-7811: Rich text modified state not detected correctly by the backend (#4982)
  Ensure convert doesn't fail on its own (#4983)
  Pro 7831 dynamic choices for piece manager filters (#4969)
  PRO-7797: revert deps (#4976)
@haroun

haroun commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

I don't think it makes sense for them to change or confuse the modifiers prop

I tried to match the screenshots in the original ticket https://apostrophetechnologies.zendesk.com/agent/tickets/1317

I just don't think it makes sense. What those modifiers do is not documented or previewable. Why don't we give them a prop to apply a class to the overridden element so that they can further refine it in their own front-end?

Good idea, I'm working on it

@boutell

boutell commented Jul 3, 2025

Copy link
Copy Markdown
Member

Please don't wait for my review on this, I think we have a good quorum of reviewers on it

@haroun

haroun commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

@stuartromanek I did the requested change with the class.

@haroun
haroun merged commit 6b3792d into main Jul 7, 2025
9 checks passed
@haroun
haroun deleted the pro-7647-custom-meta-icon branch July 7, 2025 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants